Populate asset identity from OPC UA device information#492
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for populating and surfacing Component asset-identity (“nameplate”) data, primarily from OPC UA server device information (ServerStatus/BuildInfo + optional OPC UA DI nameplate), and ensures identity/provenance are carried through discovery and REST responses. It also extends the manifest parser and REST DTOs to support/emit identity with per-field provenance.
Changes:
- Read OPC UA device-info (BuildInfo + DI DeviceSet) via
OpcuaClient, map it intoAssetIdentitywith per-fieldopcuaprovenance, and attach it to OPC UA-discovered Components. - Parse optional
identity:blocks in manifest components, pre-stamp provenance asmanifest, and emitx-medkit.identityin component list/detail REST responses. - Add unit tests for identity mapping + discovery handler JSON, plus an OPC UA fixture-based test that validates identity population/serialization end-to-end within the plugin path.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_identity.cpp | Adds fixture-based test exercising OPC UA device-info reads and plugin identity propagation/serialization. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_device_identity.cpp | Adds pure unit tests for DeviceInfo -> AssetIdentity mapping and provenance rules. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/test/fixtures/test_alarm_server/test_alarm_server.cpp | Extends OPC UA test fixture to expose deterministic BuildInfo and a minimal DI nameplate. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp | Populates component identity from live OPC UA device-info and tags component source as opcua. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp | Implements browsing/reads for BuildInfo + best-effort DI nameplate acquisition. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/device_identity.cpp | Implements mapping logic into AssetIdentity with opcua provenance stamping. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp | Adds cached AssetIdentity members for device identity in the plugin. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp | Adds DeviceInfo struct and read_device_info() API. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/device_identity.hpp | Declares the DeviceInfo -> AssetIdentity mapping helper. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt | Wires in new source + adds new unit/integration-ish gtests (device mapping + identity E2E). |
| src/ros2_medkit_gateway/test/test_manifest_parser.cpp | Adds coverage for parsing component identity blocks + manifest provenance stamping. |
| src/ros2_medkit_gateway/test/test_discovery_handlers.cpp | Adds coverage that manifest identity is emitted over list/detail responses with provenance. |
| src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp | Emits x-medkit.identity when component identity is present (list + detail endpoints). |
| src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp | Parses component identity: blocks and pre-stamps provenance as manifest. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp | Extends DTO to optionally carry identity JSON in x-medkit component payloads. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp | Adds parse_identity() declaration for manifest component identity parsing. |
cbbb5e1 to
4e2f3de
Compare
bburda
left a comment
There was a problem hiding this comment.
Additional findings outside the diff:
-
Docs: the OPC UA plugin README (
ros2_medkit_opcua/README.md) is not updated for the new auto-populatedx-medkit.identityfrom device-info, though it is the discoverable doc for OPC UA users and already documents the plugin's other capabilities. The updatedrest.rst/manifest-schema.rstdescribe the identity shape but not two residual behaviors worth stating: the OPC UA read is one-shot (not refreshed on reconnect, see the latch comment) and identity (model, software version, serial) is served unauthenticated on the default profile, i.e. a device fingerprint that only the secure profile gates. -
Lower severity, noting only:
browse_forward_children(opcua_client.cpp) drops the BrowseNext continuation point, so a large Objects/DeviceSet can be silently truncated and a device missed; anddevice_identity_/device_identity_loaded_are plain (non-atomic, unlocked) members, safe only ifintrospect()is never called concurrently.
|
Both outside-the-diff notes addressed in 7a5ca88: the OPC UA README now documents x-medkit.identity (what is read, per-session refresh on reconnect, the trust-gated precedence rule, and that identity is served unauthenticated under the default auth.enabled=false); and browse_forward_children now follows BrowseNext continuation points until exhausted (negative-verified: with the loop disabled the DI fields come back empty). |
Read ServerStatus/BuildInfo and the OPC UA DI nameplate on connect and map them onto the Component identity, feeding the discovery merge so a connected device shows its nameplate over SOVD. The PLC runtime component now carries source="opcua" (preserved by the plugin layer) so the nameplate ranks at protocol-tag precedence in the identity merge, with per-field provenance "opcua". Also parse a manifest identity block and emit x-medkit.identity on the component endpoints. Adds unit and no-HW integration tests. Refs #489
open62541 1.3 policies store the address of the config's embedded logger; moving the stack-built ClientConfig into the client leaves it dangling and the gateway segfaults on the first policy log line (SecureChannel setup). Refs #489
Fail hard when the in-package test_alarm_server fixture is missing instead of skipping, include <cstdio> for std::remove, and document x-medkit.identity on component responses plus the manifest identity block. Refs #489
…r filter The quality job's -header-filter='.*ros2_medkit.*' matches vendored headers because their install paths contain the package name, so any TU including them (e.g. via lock_manager.hpp) fails with third-party warnings. Wrap them in NOLINTBEGIN/NOLINTEND.
On slow CI runners the fired alarm does not always propagate through report -> fault_manager -> REST within 40s. Raise the wait to 120s and the CTest timeout to 420s so a failing run still prints diagnostics. Passing runs exit the poll early and are unaffected.
Only open62541pp's own interface dirs were marked SYSTEM; the bundled open62541 target's dirs stayed plain -I, so including a header like client_highlevel.h turns its old-style casts into hard clang-tidy errors via -Werror=old-style-cast.
Complete the special-member sets of OpcuaClient, OpcuaPlugin and OpcuaPoller with deleted moves, drop a no-effect std::move on the trivially copyable Subscription handle, build event-field Variants via the deep-copy constructor, and convert an index loop to range-for.
Re-read the nameplate per session via a connection generation counter, gate the opcua source tag on an authenticated channel (plugin ranks below manifest for unauthenticated reads), follow BrowseNext continuation points, and document x-medkit.identity in the README. Refs #489
7a5ca88 to
3ba16fe
Compare
open62541's UA_DateTime_localTimeUtcOffset calls glibc mktime(), whose first-call tzset() lazily inits the process-global tz cache unsynchronised. The poller and a second client race on that one-time init; narrow the suppression to the vendored/glibc frame (same class as the rcl logging one).
Populates the asset-identity fields from the connected OPC UA server's device information: ServerStatus/BuildInfo (manufacturer, product, software version, build number) and, where exposed, the standard identification nodes (Manufacturer, Model, SerialNumber, HardwareRevision, SoftwareRevision). Per-field provenance is recorded as
opcuaand the component source is set toopcua, so protocol-sourced identity ranks by the documented source precedence.Also includes a small standalone fix surfaced while running the suite: open62541 1.3.x security policies keep a pointer to the stack-built client config's logger, which dangles after the config is moved into the client (the open62541pp fixup only covers <= 1.2).
apply_security_config()now re-points the policy loggers to the client's final config; without it the secured connect can crash in non-Release builds.Tested: unit suites for the device-info mapping plus end-to-end tests against the live test server fixture (identity visible under
x-medkit.identitywith provenance over REST); the secured connect test passes with the logger fix. Validation against a real S7-1500 nameplate is planned on the bench.Closes #489